[KeyInstr] Use DISubprogram's is-key-instructions-on flag at DWARF emission #144104
+123
−5
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ission A non-key-instructions function inlined into a key-instructions function uses non-key-instructions is_stmt placement (without `findForceIsStmtInstrs`). A key-instructions function inlined into a non-key-instructions function currently results in falling back to non-key-instructions for the inlined scope too. Both of these consessions (not using `findForceIsStmtInstrs` in the 1st case, and not using Key Instructions for the inlined scope in the 2nd) are for performance reasons; to do the right thing we'd need to run both `findForceIsStmtInstrs` and `computeKeyInstructions` - in case that's controversial I've got a seperate PR for that. <link to PR>
|
@llvm/pr-subscribers-debuginfo Author: Orlando Cazalet-Hyams (OCHyams) ChangesPatch 2/4 adding bitcode support. A non-key-instructions function inlined into a key-instructions function uses non-key-instructions is_stmt placement (without A key-instructions function inlined into a non-key-instructions function currently results in falling back to non-key-instructions for the inlined scope too. Both of these consessions (not using Full diff: https://github.com/llvm/llvm-project/pull/144104.diff 2 Files Affected:
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
index 0edfca78b0886..96ae175cd632b 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
@@ -169,8 +169,10 @@ static cl::opt<DwarfDebug::MinimizeAddrInV5> MinimizeAddrInV5Option(
"Stuff")),
cl::init(DwarfDebug::MinimizeAddrInV5::Default));
-static cl::opt<bool> KeyInstructionsAreStmts("dwarf-use-key-instructions",
- cl::Hidden, cl::init(false));
+/// Set to false to ignore Key Instructions metadata.
+static cl::opt<bool> KeyInstructionsAreStmts(
+ "dwarf-use-key-instructions", cl::Hidden, cl::init(true),
+ cl::desc("Set to false to ignore Key Instructions metadata"));
static constexpr unsigned ULEB128PadSize = 4;
@@ -2077,8 +2079,17 @@ void DwarfDebug::beginInstruction(const MachineInstr *MI) {
unsigned LastAsmLine =
Asm->OutStreamer->getContext().getCurrentDwarfLoc().getLine();
+ // Not-Key-Instructions functions inlined into Key Instructions functions
+ // should use default is_stmt handling. Key Instructions functions inlined
+ // into not-key-instructions functions currently fall back to not-key
+ // handling to avoid having to run computeKeyInstructions for all functions
+ // (which will impact non-key-instructions builds).
+ // TODO: Investigate the performance impact of doing that.
+ bool ScopeUsesKeyInstructions =
+ KeyInstructionsAreStmts && DL && SP->getKeyInstructionsEnabled();
+
bool IsKey = false;
- if (KeyInstructionsAreStmts && DL && DL.getLine())
+ if (ScopeUsesKeyInstructions && DL && DL.getLine())
IsKey = KeyInstructions.contains(MI);
if (!DL && MI == PrologEndLoc) {
@@ -2158,7 +2169,7 @@ void DwarfDebug::beginInstruction(const MachineInstr *MI) {
PrologEndLoc = nullptr;
}
- if (KeyInstructionsAreStmts) {
+ if (ScopeUsesKeyInstructions) {
if (IsKey)
Flags |= DWARF2_FLAG_IS_STMT;
} else {
@@ -2651,7 +2662,13 @@ void DwarfDebug::beginFunctionImpl(const MachineFunction *MF) {
PrologEndLoc = emitInitialLocDirective(
*MF, Asm->OutStreamer->getContext().getDwarfCompileUnitID());
- if (KeyInstructionsAreStmts)
+ // If this function wasn't built with Key Instructions but has a function
+ // inlined into it that was, we treat the inlined instance as if it wasn't
+ // built with Key Instructions. If this function was built with Key
+ // Instructions but a function inlined into it wasn't then we continue to use
+ // Key Instructions for this function and fall back to non-key behaviour for
+ // the inlined function (except it doesn't beneit from findForceIsStmtInstrs).
+ if (KeyInstructionsAreStmts && SP->getKeyInstructionsEnabled())
computeKeyInstructions(MF);
else
findForceIsStmtInstrs(MF);
diff --git a/llvm/test/DebugInfo/KeyInstructions/X86/dwarf-inline-modes.mir b/llvm/test/DebugInfo/KeyInstructions/X86/dwarf-inline-modes.mir
new file mode 100644
index 0000000000000..e304b8fd8b00a
--- /dev/null
+++ b/llvm/test/DebugInfo/KeyInstructions/X86/dwarf-inline-modes.mir
@@ -0,0 +1,98 @@
+# RUN: llc %s --start-after=livedebugvalues --dwarf-use-key-instructions --filetype=obj -o - \
+# RUN: | llvm-objdump -d - --no-show-raw-insn \
+# RUN: | FileCheck %s --check-prefix=OBJ
+
+# RUN: llc %s --start-after=livedebugvalues --dwarf-use-key-instructions --filetype=obj -o - \
+# RUN: | llvm-dwarfdump - --debug-line \
+# RUN: | FileCheck %s --check-prefix=DBG
+
+
+
+--- |
+ target triple = "x86_64-unknown-linux-gnu"
+
+ define hidden noundef i32 @key() local_unnamed_addr !dbg !5 {
+ entry:
+ ret i32 0
+ }
+
+ define hidden noundef i32 @not_key() local_unnamed_addr !dbg !9 {
+ entry:
+ ret i32 0
+ }
+
+ !llvm.dbg.cu = !{!0}
+ !llvm.module.flags = !{!2, !3}
+ !llvm.ident = !{!4}
+
+ !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_17, file: !1, producer: "clang version 21.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: LineTablesOnly, splitDebugInlining: false, nameTableKind: None)
+ !1 = !DIFile(filename: "test.cpp", directory: "/")
+ !2 = !{i32 7, !"Dwarf Version", i32 5}
+ !3 = !{i32 2, !"Debug Info Version", i32 3}
+ !4 = !{!"clang version 21.0.0"}
+ !5 = distinct !DISubprogram(name: "key", scope: !1, file: !1, line: 1, type: !6, scopeLine: 1, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, keyInstructions: true)
+ !6 = !DISubroutineType(types: !7)
+ !7 = !{}
+ !9 = distinct !DISubprogram(name: "not_key", scope: !1, file: !1, line: 1, type: !6, scopeLine: 1, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, keyInstructions: false)
+ !10 = distinct !DILocation(line: 5, scope: !5)
+ !11 = distinct !DILocation(line: 9, scope: !9)
+...
+---
+name: key
+alignment: 16
+body: |
+ bb.0.entry:
+
+ ; OBJ: 0000000000000000 <key>:
+ ; OBJ-NEXT: 0: movl $0x1, %eax
+ ; OBJ-NEXT: 5: movl $0x2, %eax
+ ; OBJ-NEXT: a: movl $0x3, %eax
+ ; OBJ-NEXT: f: movl $0x4, %eax
+ ; OBJ-NEXT: 14: movl $0x5, %eax
+ ; OBJ-NEXT: 19: retq
+ ;
+ ; DBG: Address Line Column File ISA Discriminator OpIndex Flags
+ ; DBG-NEXT: ------------------ ------ ------ ------ --- ------------- ------- -------------
+ ; DBG-NEXT: 0x0000000000000000 1 0 0 0 0 0 is_stmt prologue_end
+ ; DBG-NEXT: 0x0000000000000005 2 0 0 0 0 0 is_stmt
+ ; DBG-NEXT: 0x000000000000000a 2 0 0 0 0 0
+ ; DBG-NEXT: 0x000000000000000f 3 0 0 0 0 0 is_stmt
+ ; DBG-NEXT: 0x0000000000000014 3 0 0 0 0 0
+ ;
+ $eax = MOV32ri 1, debug-location !DILocation(line: 1, scope: !5) ; is_stmt (prologue_end)
+ $eax = MOV32ri 2, debug-location !DILocation(line: 2, scope: !5, atomGroup: 1, atomRank: 1) ; is_stmt (key)
+ $eax = MOV32ri 3, debug-location !DILocation(line: 2, scope: !9, inlinedAt: !10)
+ $eax = MOV32ri 4, debug-location !DILocation(line: 3, scope: !9, inlinedAt: !10) ; is_stmt (not_key)
+ $eax = MOV32ri 5, debug-location !DILocation(line: 3, scope: !5, atomGroup: 1, atomRank: 2) ; is_stmt (key)
+ RET64 $eax, debug-location !DILocation(line: 3, scope: !5, atomGroup: 1, atomRank: 1)
+...
+---
+name: not_key
+alignment: 16
+body: |
+ bb.0.entry:
+
+ ; OBJ: 0000000000000020 <not_key>:
+ ; OBJ-NEXT: 20: movl $0x1, %eax
+ ; OBJ-NEXT: 25: movl $0x2, %eax
+ ; OBJ-NEXT: 2a: movl $0x3, %eax
+ ; OBJ-NEXT: 2f: retq
+ ;
+ ; TODO: Currently key inlined into not-key is treated as not-key. Investigate
+ ; performance implications of honouring the flag in this scenario.
+ ;
+ ; Address Line Column File ISA Discriminator OpIndex Flags
+ ; ------------------ ------ ------ ------ --- ------------- ------- -------------
+ ; DBG-NEXT: 0x0000000000000020 1 0 0 0 0 0 is_stmt prologue_end
+ ; DBG-NEXT: 0x0000000000000025 2 0 0 0 0 0 is_stmt
+ ; DBG-NEXT: 0x000000000000002a 3 0 0 0 0 0 is_stmt
+ ; DBG-NEXT: 0x000000000000002f 3 0 0 0 0 0
+ ;
+ ; NOTE: The `is_stmt` comments at the end of the lines reflects what we want
+ ; to see if the TODO above is resolved.
+ ;
+ $eax = MOV32ri 1, debug-location !DILocation(line: 1, scope: !9) ; is_stmt (prologue_end)
+ $eax = MOV32ri 2, debug-location !DILocation(line: 2, scope: !5, inlinedAt: !11, atomGroup: 1, atomRank: 2)
+ $eax = MOV32ri 3, debug-location !DILocation(line: 3, scope: !5, inlinedAt: !11, atomGroup: 1, atomRank: 1) ; is_stmt (key)
+ RET64 $eax, debug-location !DILocation(line: 3, scope: !9)
+...
|
This was referenced Jun 13, 2025
OCHyams
added a commit
to OCHyams/llvm-project
that referenced
this pull request
Jun 13, 2025
Once llvm#144104 lands the flag is true by default (because each DISubprogram will track whether or not it's using key instructions).
This was referenced Jun 13, 2025
jmorse
reviewed
Jun 23, 2025
| @@ -2077,8 +2079,17 @@ void DwarfDebug::beginInstruction(const MachineInstr *MI) { | |||
| unsigned LastAsmLine = | |||
| Asm->OutStreamer->getContext().getCurrentDwarfLoc().getLine(); | |||
|
|
|||
| // Not-Key-Instructions functions inlined into Key Instructions functions | |||
| // should use default is_stmt handling. Key Instructions functions inlined | |||
There was a problem hiding this comment.
I think the word "default" here is liable to become stale, is there another phrase we can use or invent? Keyless?
| @@ -2077,8 +2079,17 @@ void DwarfDebug::beginInstruction(const MachineInstr *MI) { | |||
| unsigned LastAsmLine = | |||
| Asm->OutStreamer->getContext().getCurrentDwarfLoc().getLine(); | |||
|
|
|||
There was a problem hiding this comment.
This is an immense nitpick, but IMO the comment should open by stating the premise "There may be a mixture of scopes using KIs, and not using KIs" or something similar. It's sufficiently non-obvious that it should be made explicit to the future reader.
| // built with Key Instructions. If this function was built with Key | ||
| // Instructions but a function inlined into it wasn't then we continue to use | ||
| // Key Instructions for this function and fall back to non-key behaviour for | ||
| // the inlined function (except it doesn't beneit from findForceIsStmtInstrs). |
There was a problem hiding this comment.
Suggested change
| // the inlined function (except it doesn't beneit from findForceIsStmtInstrs). | |
| // the inlined function (except it doesn't benefit from findForceIsStmtInstrs). |
| # RUN: | llvm-dwarfdump - --debug-line \ | ||
| # RUN: | FileCheck %s --check-prefix=DBG | ||
|
|
||
|
|
There was a problem hiding this comment.
Comment as to the intention of the test please (mixing different KI-ness of scopes?)
Tests in that directory are only run if (LLVM_)EXPERIMENTAL_KEY_INSTRUCTIONS is enabled, so that's not an issue. |
|
I've addressed the review comments |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Patch 2/4 adding bitcode support.
A non-key-instructions function inlined into a key-instructions function uses non-key-instructions is_stmt placement (without
findForceIsStmtInstrs).A key-instructions function inlined into a non-key-instructions function currently results in falling back to non-key-instructions for the inlined scope too.
Both of these concessions (not using
findForceIsStmtInstrsin the 1st case, and not using Key Instructions for the inlined scope in the 2nd) are for performance reasons; to do the right thing we'd need to run bothfindForceIsStmtInstrsandcomputeKeyInstructions- in case that's controversial I've got a separate PR for that: #144103.